Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Game 18568 fast note creation ownership #72

Open
wants to merge 6 commits into
base: dynamite-lib
Choose a base branch
from

Conversation

eliveikis
Copy link
Contributor

SG3DR needs to immediately capture the application state when a note is created, not wait until the shotgun-id is available.

When a note is created by the application, this change adds the ability for the application to pass a userdata object along with the creation request that is returned in the callback for that note creation.

When a note is created by the toolkit, e.g. by the user using the NoteInputWidget, new signals have been added to immediately notify the application that a note is being create, and passing note creation task Ids with the creation request and when the note is finally created.

These two pathways allow the application to synchronize and match requests for new notes with the creation confirmation callbacks.

This fixes bugs when rapidly creating notes where the application state would change and shotgun notes would become queued and take a long time to create, resulting in state mismatch of when the note was originally created.

@eliveikis
Copy link
Contributor Author

@gomezll Please review, needs cleanup and I'll point out an issue in the comments...
This enables the changes in:
https://github.com/shotgunsoftware/tk-multi-versiondetails/pull/23

# Emitted when a new note or reply will be created. The creation request id
# is sent and will match the id sent with the entity_created signal.
# Todo: This has functional overlap with the pre_submit_callback...resolve that...
new_entity_requested = QtCore.Signal(str, int)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gomezll I added these extra signals and functionality before I realized there is already a mechanism that supplies similar functionality - the pre_submit_callback. Imo the existing callback is a bit odd since it passes the NoteInputWidget object as its parameter which feels a bit arbitrary, and some work would be needed to use that one since the pending request UID would need to be exposed through a function and that also seems a bit random.

I took the approach of adding new signals specific for the exact event type that we're trying to track (shotgun note creations and external user creations (in activity_stream.py), which seems nice and explicit but also makes things a bit busier.

Interested to see what you think.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your approach with new signals seems justified and appropriate to me Ed. Looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants